Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release-4.14] OCPBUGS-11550: AUTH: update cluster-reader to include k8s.ovn.org #1791

Merged
merged 1 commit into from
Apr 26, 2023
Merged

[release-4.14] OCPBUGS-11550: AUTH: update cluster-reader to include k8s.ovn.org #1791

merged 1 commit into from
Apr 26, 2023

Conversation

flavio-fernandes
Copy link
Contributor

API group "k8s.ovn.org" should be included to cluster-reader role. That group has the following resources:
- EgressFirewall
- EgressIP
- EgressQoS

@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Apr 24, 2023
@openshift-ci-robot
Copy link
Contributor

@flavio-fernandes: This pull request references Jira Issue OCPBUGS-11550, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @anuragthehatter

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

API group "k8s.ovn.org" should be included to cluster-reader role. That group has the following resources:

  • EgressFirewall
  • EgressIP
  • EgressQoS

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@flavio-fernandes flavio-fernandes mentioned this pull request Apr 25, 2023
Closed
@flavio-fernandes
rbac: ovn-kubernetes: add aggregate-to-cluster-reader permissions
7f2f900 
API group "k8s.ovn.org" should be included to cluster-reader role.
That group has the following resources:
    - EgressFirewall
    - EgressIP
    - EgressQoS

Signed-off-by: Flavio Fernandes <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 25, 2023

@flavio-fernandes: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-network-mtu-migration-ovn-ipv4 7f2f900 link false /test e2e-network-mtu-migration-ovn-ipv4
ci/prow/e2e-gcp-ovn-upgrade 7f2f900 link false /test e2e-gcp-ovn-upgrade
ci/prow/e2e-openstack-ovn 7f2f900 link false /test e2e-openstack-ovn
ci/prow/e2e-vsphere-ovn 7f2f900 link false /test e2e-vsphere-ovn

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@flavio-fernandes
Copy link
Contributor Author

Verified this change by deploying OCP with the new cluster role:

vagrant@devmaster ~/dev/openshift-apiserver.git OCPBUGS-11550_k8s.ovn.org*
❯ oc auth can-i list egressfirewalls --as=test --as-group=mylocaladmins
no

❯ oc --user=admin create secret generic htpasswd --from-file=htpasswd -n openshift-config
secret/htpasswd created

❯ oc replace -f - <<API
apiVersion: config.openshift.io/v1
kind: OAuth
metadata:
  name: cluster
spec:
  identityProviders:
  - name: Local Password
    mappingMethod: claim
    type: HTPasswd
    htpasswd:
      fileData:
        name: htpasswd
API
oauth.config.openshift.io/cluster replaced

❯ oc adm groups new mylocaladmins
group.user.openshift.io/mylocaladmins created

❯ oc adm groups add-users mylocaladmins test
group.user.openshift.io/mylocaladmins added: "test"

❯ oc adm policy add-cluster-role-to-group cluster-reader mylocaladmins
clusterrole.rbac.authorization.k8s.io/cluster-reader added: "mylocaladmins"

❯ oc auth can-i create pods --as=test --as-group=mylocaladmins
no

❯ oc auth can-i list pods --as=test --as-group=mylocaladmins
yes

❯ oc auth can-i list egressips --as=test --as-group=mylocaladmins
Warning: resource 'egressips' is not namespace scoped in group 'k8s.ovn.org'

yes

❯ oc auth can-i list egressfirewalls --as=test --as-group=mylocaladmins
yes

❯ oc auth can-i create egressfirewalls --as=test --as-group=mylocaladmins
no

❯ oc describe ClusterRole openshift-ovn-kubernetes-cluster-reader
Name:         openshift-ovn-kubernetes-cluster-reader
Labels:       rbac.authorization.k8s.io/aggregate-to-cluster-reader=true
Annotations:  <none>
PolicyRule:
  Resources                    Non-Resource URLs  Resource Names  Verbs
  ---------                    -----------------  --------------  -----
  egressfirewalls.k8s.ovn.org  []                 []              [get list watch]
  egressips.k8s.ovn.org        []                 []              [get list watch]
  egressqoses.k8s.ovn.org      []                 []              [get list watch]

❯ oc get ClusterRole cluster-reader -oyaml | grep -B1 -A9 ovn
- apiGroups:
  - k8s.ovn.org
  resources:
  - egressfirewalls
  - egressips
  - egressqoses
  verbs:
  - get
  - list
  - watch
- apiGroups:

@flavio-fernandes
Copy link
Contributor Author

Some cool references on the aggregation support:

@flavio-fernandes
Copy link
Contributor Author

@kyrtapz PTAL

@flavio-fernandes
Copy link
Contributor Author

flavio-fernandes commented Apr 25, 2023 •
edited
Loading

Original change was done at the wrong (not as good) place:
openshift/openshift-apiserver#363

@kyrtapz
Copy link
Contributor

kyrtapz commented Apr 26, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2023
trozet
trozet approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@trozet trozet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flavio-fernandes, kyrtapz, trozet

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2023
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD ddb4322 and 2 for PR HEAD 7f2f900 in total

@openshift-merge-robot openshift-merge-robot merged commit 6d3d4df into openshift:master Apr 26, 2023
@openshift-ci-robot
Copy link
Contributor

@flavio-fernandes: Jira Issue OCPBUGS-11550: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-11550 has been moved to the MODIFIED state.

In response to this:

API group "k8s.ovn.org" should be included to cluster-reader role. That group has the following resources:

  • EgressFirewall
  • EgressIP
  • EgressQoS

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@flavio-fernandes flavio-fernandes deleted the OCPBUGS-11550_k8s.ovn.org branch April 27, 2023 13:47
@flavio-fernandes
Copy link
Contributor Author

/cherry-pick release-4.13

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Apr 27, 2023
Merged
@openshift-cherrypick-robot
Copy link

@flavio-fernandes: new pull request created: #1797

In response to this:

/cherry-pick release-4.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trozet trozet trozet approved these changes

@anuragthehatter anuragthehatter Awaiting requested review from anuragthehatter

@abhat abhat Awaiting requested review from abhat

@dougbtv dougbtv Awaiting requested review from dougbtv

Assignees

@kyrtapz kyrtapz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@flavio-fernandes @openshift-ci-robot @kyrtapz @openshift-cherrypick-robot @trozet @openshift-merge-robot